Skip to content

[codex] Keep native gateway out of WSL recovery#771

Closed
ArtLupo wants to merge 1 commit into
openclaw:mainfrom
ArtLupo:codex/native-gateway-wsl-recovery
Closed

[codex] Keep native gateway out of WSL recovery#771
ArtLupo wants to merge 1 commit into
openclaw:mainfrom
ArtLupo:codex/native-gateway-wsl-recovery

Conversation

@ArtLupo

@ArtLupo ArtLupo commented Jun 17, 2026

Copy link
Copy Markdown

Summary

Keeps Windows-native local gateways out of the WSL/Docker recovery path.

What changed

  • WslKeepAlivePolicy.ShouldStart now starts WSL keepalive only for setup-managed local gateway records.
  • SetupExistingGatewayClassifier now treats only setup-managed local records, or valid setup-state evidence, as app-owned WSL setup evidence.
  • Added regression tests for native loopback gateways such as Desktop-A Native Gateway.

Why

A Windows-native gateway can be local and loopback-bound without being managed by the old WSL/Docker setup. Treating every local loopback record as app-owned WSL evidence can incorrectly restart or recover the WSL/Docker path after the user has moved to the native Windows gateway.

Validation

  • dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --filter "FullyQualifiedName~WslKeepAlivePolicyTests|FullyQualifiedName~StartupSetupStateTests": passed, 45 tests
  • .\build.ps1: passed
  • dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore: passed, 1075 tests
  • dotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restore: exited 0
  • git diff --check: passed

@clawsweeper

clawsweeper Bot commented Jun 17, 2026 •
edited
Loading

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed June 17, 2026, 5:20 AM ET / 09:20 UTC.

Summary
The PR narrows WSL keepalive and setup classification to setup-managed local gateway records, with regression tests for native loopback gateways.

Reproducibility: yes. from source inspection: current main returns true for any active local loopback record in WslKeepAlivePolicy.ShouldStart and treats any local loopback registry record as setup evidence in SetupExistingGatewayClassifier. I did not run the Windows tray path because this review is read-only and the environment is not the real Windows runtime.

Review metrics: 1 noteworthy metric.

  • Touched surface: 4 files changed, 48 added, 15 removed. The PR is tightly scoped to WSL ownership policy and regression tests.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Add redacted runtime proof showing a native loopback gateway no longer starts WSL recovery, preferably with tray logs or terminal output from startup.
  • [P1] Add or cite upgrade proof that a migrated/setup-managed WSL gateway still starts keepalive after this change.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body lists build and test validation but does not include after-fix real runtime proof from a native loopback gateway setup; screenshots, terminal output, copied live output, linked artifacts, recordings, or redacted logs would count, and private details should be redacted. After proof is added to the PR body, a fresh ClawSweeper review should run automatically; otherwise a maintainer can comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] The PR intentionally stops WSL keepalive for legacy local URL settings when no active setup-managed registry record exists; maintainers should confirm migration/setup-state coverage is enough for upgrades.
  • [P1] GitHub reports no status checks for this draft PR, so merge readiness currently depends on contributor-reported validation plus review.

Maintainer options:

  1. Require upgrade proof for legacy WSL users (recommended)
    Ask for proof that a legacy WSL setup with migrated registry or setup-state evidence still starts keepalive after this change.
  2. Accept the narrowed recovery behavior
    Maintainers may decide that no-active-record legacy local URLs are obsolete enough to fail closed now that registry migration owns credentials.

Next step before merge

  • [P1] Contributor action is real behavior proof plus maintainer confirmation of the legacy-local-url upgrade tradeoff; there is no narrow code defect to queue for automated repair.

Security
Cleared: The diff touches only tray WSL classification code and unit tests, with no dependency, CI, credential, or code-download changes.

Review details

Best possible solution:

Keep WSL recovery tied to explicit setup-managed records or setup-state evidence while preserving documented legacy WSL ownership markers where upgrades still need them.

Do we have a high-confidence way to reproduce the issue?

Yes from source inspection: current main returns true for any active local loopback record in WslKeepAlivePolicy.ShouldStart and treats any local loopback registry record as setup evidence in SetupExistingGatewayClassifier. I did not run the Windows tray path because this review is read-only and the environment is not the real Windows runtime.

Is this the best way to solve the issue?

Mostly yes; reusing IsSetupManagedLocalRecord is the narrowest maintainable source fix, but the no-active-record legacy fallback should be covered by upgrade proof before merge.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against d392f7d4ea0d.

Label changes

Label changes:

  • add P2: Incorrect WSL recovery can restart or stop the local gateway path for a limited set of Windows gateway users.
  • add merge-risk: 🚨 compatibility: The PR changes upgrade behavior for users whose local gateway is represented only by legacy settings rather than an active setup-managed registry record.
  • add merge-risk: 🚨 availability: A wrong keepalive decision can either restart stale WSL recovery or fail to keep an app-owned WSL gateway alive across tray exits.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body lists build and test validation but does not include after-fix real runtime proof from a native loopback gateway setup; screenshots, terminal output, copied live output, linked artifacts, recordings, or redacted logs would count, and private details should be redacted. After proof is added to the PR body, a fresh ClawSweeper review should run automatically; otherwise a maintainer can comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P2: Incorrect WSL recovery can restart or stop the local gateway path for a limited set of Windows gateway users.
  • merge-risk: 🚨 compatibility: The PR changes upgrade behavior for users whose local gateway is represented only by legacy settings rather than an active setup-managed registry record.
  • merge-risk: 🚨 availability: A wrong keepalive decision can either restart stale WSL recovery or fail to keep an app-owned WSL gateway alive across tray exits.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body lists build and test validation but does not include after-fix real runtime proof from a native loopback gateway setup; screenshots, terminal output, copied live output, linked artifacts, recordings, or redacted logs would count, and private details should be redacted. After proof is added to the PR body, a fresh ClawSweeper review should run automatically; otherwise a maintainer can comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

Acceptance criteria:

  • [P1] ./build.ps1.
  • [P1] dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore.
  • [P1] dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore.

What I checked:

Likely related people:

  • indierawk2k2: History shows the WSL local gateway onboarding and WSL keepalive behavior date through commits adding WSL onboarding and keepalive support. (role: feature-history contributor; confidence: high; commits: 581f78d276e1, 4baf01d768c0; files: src/OpenClaw.Tray.WinUI/Services/WslKeepAlivePolicy.cs, src/OpenClaw.Tray.WinUI/Services/SetupExistingGatewayClassifier.cs)
  • ranjeshj: Recent history includes setup-managed WSL gateway behavior changes and SetupEngine refactor work touching the affected setup/WSL ownership area. (role: recent area contributor; confidence: medium; commits: cefce3952ab1, f4d4793f744f; files: src/OpenClaw.Tray.WinUI/Services/WslKeepAlivePolicy.cs, src/OpenClaw.Tray.WinUI/Services/SetupExistingGatewayClassifier.cs, src/OpenClaw.SetupEngine/SetupSteps.cs)
  • AlexAlves87: History shows the keepalive service was recently extracted from App.xaml.cs, making this person relevant for service-level regressions. (role: recent refactor contributor; confidence: medium; commits: b175439fbfd4; files: src/OpenClaw.Tray.WinUI/Services/WslGatewayKeepAliveService.cs, src/OpenClaw.Tray.WinUI/App.xaml.cs)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. labels Jun 17, 2026
@shanselman

shanselman commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

@ArtLupo this is marked draft, how are you feeling about this PR?

@ArtLupo ArtLupo closed this Jun 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ArtLupo @shanselman